Skip to content

Conversation

@BastienRodz
Copy link
Contributor

@BastienRodz BastienRodz commented Aug 28, 2024

Summary

This PR addresses issue #113 by adding schema validation to the Jobs.run method. The update ensures that the configuration object passed to Jobs.run is validated against a predefined schema, preventing potential misuse or typos in the configuration.

Changes Made

  • Added JobRunConfigSchema:

    • Defined a schema using Match.Maybe to validate optional fields such as in, on, date, data, priority, unique, singular, and callback.
    • The schema is applied to the configuration object passed to Jobs.run.
  • Updated Jobs.run Method:

    • Added a validation step to check the configuration object against JobRunConfigSchema.
    • Ensured that the method functions correctly even if no configuration object is provided.
  • Test Suite Updates:

    • Added new test cases to cover the following scenarios:
      • Running Jobs.run with only the job name.
      • Running Jobs.run with a valid configuration object.
      • Handling of invalid configurations (e.g., typos in keys).
      • Ensuring unsupported keys are rejected.

Testing

  • Ran existing tests to ensure no regressions.
  • Added new tests to validate the schema and ensure that errors are raised for invalid configurations.
  • Edited basic.js tests after adding configSchemaValidation tests.
  • Verified that all tests pass successfully.

Issue Link

Resolves #113.

Additional Notes

  • This update is based on harryadel:async-migration branch, it needs to be first merged onto master.
  • This update maintains backward compatibility, as the configuration object remains optional. If no configuration is provided, Jobs.run will continue to function as expected.
  • The test coverage has been improved to cover potential edge cases related to configuration validation.

Please review the changes and provide any feedback. Thank you!

@BastienRodz BastienRodz changed the title Add schema validation to Jobs.run and update tests Add schema validation to Jobs.run and update tests (need Async migration) Aug 28, 2024
@BastienRodz BastienRodz marked this pull request as ready for review August 28, 2024 17:34
@BastienRodz BastienRodz force-pushed the schema-validation branch 2 times, most recently from bf71a3b to 5937452 Compare November 2, 2024 12:59
@BastienRodz
Copy link
Contributor Author

@msavin @StorytellerCZ This is ready to merge, I rebased the branch on the new master version.

Copy link
Contributor

@StorytellerCZ StorytellerCZ left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM on the first look.

@BastienRodz
Copy link
Contributor Author

Hi @msavin, just wanted to kindly check if there’s anything else needed from my side to get this PR merged. Let me know if there’s anything I can do to help! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants